Skip to content

feat(tci): log the process behind local TCI clients. Principle VIII. - #5130

Merged
ten9876 merged 14 commits into
aethersdr:mainfrom
skerker:feat/5087-tci-peer-process
Sep 10, 2026
Merged

feat(tci): log the process behind local TCI clients. Principle VIII.#5130
ten9876 merged 14 commits into
aethersdr:mainfrom
skerker:feat/5087-tci-peer-process

Conversation

@skerker

@skerker skerker commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #5087

TCI has no client-identification message and the WebSocket handshake is a bare
upgrade, so TciServer: client connected from "::ffff:127.0.0.1" was all a
support bundle ever held about a misbehaving client. For the common same-machine
case the OS knows which process owns that socket; this asks it.

What changed:

  • New src/core/TciPeerProcess.{h,cpp}resolveLoopbackPeerProcess(peerAddr, peerPort): pure OS lookup, no Qt networking beyond QHostAddress. Linux:
    /proc/net/tcp6 then /proc/net/tcp (a loopback IPv4 client on the Any-bound
    listener appears v4-mapped in tcp6) → socket inode → /proc/*/fdcomm +
    exe. macOS: proc_listpidsPROC_PIDLISTFDSPROC_PIDFDSOCKETINFO
    matching the client's local port/address → proc_name + proc_pidpath.
    Windows: GetExtendedTcpTable(TCP_TABLE_OWNER_PID_ALL) for AF_INET and AF_INET6
    QueryFullProcessImageNameW; version from the exe's authored StringFileInfo
    strings — ProductVersion first (it is where build metadata lives: WSJT-X's reads
    "3.0.1 c04dd8"), then FileVersion, trying the declared Translation pairs plus the
    standard Unicode blocks 040904b0/000004b0 (real exes ship Translation entries that
    don't match their actual block — measured on WSJT-X, Windows proof below; .NET's
    FileVersionInfo carries the same fallbacks); the numeric VS_FIXEDFILEINFO quad
    only as a last resort — its always-4-part shape is rewritten by the log
    sanitizer's IPv4 rule, authored strings are not. macOS: the bundle's Info.plist,
    found by walking up from …/Foo.app/Contents/MacOS/foo — a file read, never an
    execution of the client; reported as CFBundleShortVersionString plus
    CFBundleVersion in the form "3.0.1 (123)" when the build number differs. Linux:
    an ELF binary embeds no version; for distro-installed clients the dpkg database,
    read as plain files (/var/lib/dpkg/info/*.list maps the exe path to its package,
    /var/lib/dpkg/status maps the package to its version, which carries the
    packaging build, e.g. "2.6.1+repack-2build1"), supplies it — never a package
    tool executed; home-built binaries match no .list and stay version-less; rpm's
    database is not plain text, so non-dpkg distros remain a follow-up.
    Non-loopback peers and every failure return unresolved.
  • 26d7ebfb (found by the Linux leg of the per-platform proof below): the Linux
    fd sweep now compares /proc/<pid>/fd link targets by raw readlink(2).
    QFile::symLinkTarget() absolutizes a relative-looking target against the
    link's directory, so the raw socket:[N] came back as
    /proc/<pid>/fd/socket:[N] and the comparison never matched — the Linux
    resolver resolved nothing, ever. macOS/Windows use native APIs, untouched.
  • TciServer::onNewConnection() logs today's line unchanged, then
    resolvePeerProcess() runs the lookup on QtConcurrent with a
    QFutureWatcher — the descriptor sweep is unbounded and must not delay
    sendInitBurst(). When it lands, a second line:
    TciServer: client ::ffff:127.0.0.1:51234 process="wsjtx"
    (+ version="…" when known), and clientsChanged() fires. The executable path
    is never logged; it is kept in memory for the Network Diagnostics tooltip only
    (maintainer ruling, 2026-09-07). A non-loopback peer
    gets one qCDebug saying identity is unavailable, so the absent field is
    self-explaining in a bundle. Identity is kept on ClientState/TciClientInfo.
  • Network Diagnostics → TCI clients table: the endpoint cell reads
    127.0.0.1:51234 (wsjtx) with exe path (+ version) as its tooltip. No new
    column; nothing changes when unresolved.
  • No unit test. The lookup's positive claim (a loopback socket resolves to its
    owning process) needs a real socket, and the self-connected QTcpServer test
    that carried it was removed in 9476eb04: AGENTS.md routes positive convergence
    to the automation bridge, and the three-platform bundles below prove it with
    real clients. The negative guard (remote peer, port 0 never resolve) is a
    one-line check documented at the top of resolveLoopbackPeerProcess().
  • CMake: new .cpp in the aethercore list; Windows links iphlpapi version.

Limitations

What is collected. Only the one process that opened a TCP connection to the TCI port, and only three fields: its short name, its executable path, and a best-effort version. The log line carries the name and version; the path appears only in the Network Diagnostics tooltip, in-app. The version comes from file reads scoped to that one executable (Info.plist, the dpkg status file, the exe's resource strings) — nothing is executed. Remote peers get nothing. On Linux and macOS the lookup briefly reads other same-user processes' descriptor lists to find the match (what lsof -i does); nothing from that walk is kept. The log is local and leaves the machine only if the user shares a bundle. Privacy text, as the maintainer proposed: "When a TCI client connects from this computer, AetherSDR records the connecting program's name and version in its local log."

Packaged clients now report a version — with build metadata where the packager
authored it — on all three platforms. The version is read from what the OS or
package manager records about the binary; the client is never executed. The
honest remainder:

  • A home-built client reports name + exe but no version, on every platform.
    A bare binary's version usually exists only as compiled-in strings (fldigi's
    Help→Build info stamp, for example), which no metadata query can see — so for
    self-built clients the enhancement: log which local process is behind each TCI client connection — name, executable, version when discoverable #5087 support question ("which version is the user
    running?") still comes back empty.
  • macOS reports the bundle's Info.plist keys; a build hash appears only if the
    packager put one there (WSJT-X's mac plist carries v3.0.2/3.0.2, not the
    ccdfaf its own About shows).
  • Linux covers dpkg-owned binaries; rpm-based distros are a stated follow-up.
  • Same-user loopback clients only, by design (above).

Constitution principle honored

Principle VIII — Evidence Over Assertion: the bundle now records which program
was connected instead of the operator reconstructing it from memory.

Test plan

  • Local build passes — clean configure + full build, macOS (Intel, Qt 6.8.3): 2952/2952 clean, then reconfigure+rebuild at the commit
  • Behavior verified on a real radio if applicable — n/a, no radio involvement
    (the TCI server listens without one); verified with a live local client via
    the agent automation bridge, below
  • Existing tests pass (CI) — CI compiles all three backends (build /
    check-macos / check-windows). This PR registers no test (see "What
    changed"); the proof is the bridge bundles below
  • Reproduction steps documented — n/a (enhancement)

Proof — b3d25350 on three platforms (agent automation bridge)

The quoted lines below were read at b3d25350; as of ad427370 the log line has no exe= field (the tooltip still carries the path).

Each platform leg: clean configure + full build at b3d25350, Help→About SHA
verified (screenshots in the zips), real local clients over ws://…:50001,
radio (where connected) RX only — nothing keyed. Readings verbatim from the
attached complete unedited logs:

Platform Client Reading
Windows 11 WSJT-X 3.0.1 (installed exe) TciServer: client ::1:61576 process="wsjtx" exe="C:\WSJT\wsjtx\bin\wsjtx.exe" version="3.0.1 c04dd8" — authored ProductVersion: version AND build hash, unmasked, 1 ms after connect
Linux wsjtx 3.0.1 (dpkg-owned) TciServer: client ::ffff:127.0.0.1:59908 process="wsjtx" exe="/usr/bin/wsjtx" version="3.0.1" — the dpkg-recorded version (independently confirmed on the box: dpkg-query -W wsjtx3.0.1), 116 ms
Linux fldigi (home-built) process="fldigi" exe="…/fldigi-tci/src/fldigi" — no version= clause (owned by no package — the designed version-less path), 159 ms
macOS WSJT-X 3.0.2 (Info.plist keys differ: v3.0.2 vs 3.0.2) TciServer: client ::1:50159 process="wsjtx" exe="/Applications/wsjtx.app/Contents/MacOS/wsjtx" version="v3.0.2 (3.0.2)" — both keys, short (build) form, 11 ms
macOS Python.app 3.14.4 (bare WebSocket upgrade; plist keys equal) process="Python" exe="…/Python.app/Contents/MacOS/Python" version="3.14.4" — equal keys de-duplicate to the short version alone
macOS fldigi (home-built, no bundle) process="fldigi" exe="…/fldigi-tci/src/fldigi" — no version= clause: a bare binary carries no OS-queryable version metadata (its git stamp is a compiled-in string, visible only in its own Help→Build info)

Network
Diagnostics → TCI Clients shows the endpoint cell as 127.0.0.1:<port> (wsjtx)
with exe path (+ version) in the tooltip:

Network Diagnostics — TCI Clients, endpoint cell with the resolved process

How the per-platform legs got here (before-readings live in the attached zips):

  • Linux (26d7ebfb): at cfb74fa2 no Linux client ever resolved — the
    QFile::symLinkTarget() defect under "What changed"; the unit test failed
    6 checks on the box.
  • Windows (1897df2f, b3d25350): the numeric-quad-only read rendered every
    Windows version sanitizer-masked (version="*.*.*. 0"), and WSJT-X declares a
    \VarFileInfo\Translation block it doesn't use — both measured with the
    version-resource probe in the Windows zip, both described under "What changed".
  • macOS: bundle metadata carries no git hash (WSJT-X's own About shows
    v3.0.2 ccdfaf; its plist carries v3.0.2/3.0.2) — stated under Limitations.

Evidence zips (complete unedited logs, About screenshots, unit-test outputs,
probe/client scripts):

aethersdr-pr5130-linux-version-2026-08-24.zip

aethersdr-pr5130-windows-d2-2026-08-24.zip

aethersdr-pr5130-mac-version-2026-08-24.zip

aethersdr-pr5130-ruling-revision-2026-09-08.zip — head d1e69c7e (About SHA, Python client log line without exe=, TCI Clients cell, tci_peer_process_test 12/12, quit-with-dialog-open trace), sha256 c8fdb2a4b6b76f2d1539c28683a62aef7d7db91aaf1200efaabf757993b8da4b

Review round (2026-09-08). ad427370 name + version only in the log; d1e69c7e macOS sweep limited to this user's pids. Maintainer ruling 2026-09-07. 74a92bf7 then 9476eb04: the socket-owning unit test first met the AGENTS.md disclosure/exit-77 obligations, then was removed — the bridge bundles are the proof.

Checklist

  • Commits are signed
  • No new flat-key AppSettings calls — n/a, no settings
  • Code is clean-room
  • All meter UI uses MeterSmoothern/a
  • Documentation updated if user-visible behavior changed — n/a, diagnostic
    log line + one table cell; described above
  • Security-sensitive changes reference a GHSA — n/a. Logged data is the
    name/path of a program that connected to us on the operator's own machine
    (what ss -tp/lsof show any local user); same-user processes only.

— authored by agent (Claude Code) on behalf of @skerker

Review round (2026-09-10), 7ff41b5e. Every finding and nit from the second review pass, plus a merge of origin/main:

  • findSocketInode() skips zero-inode (TIME_WAIT/orphaned) rows instead of returning them — measured 6/6 misses before, 6/6 resolves after, with a client that reused its source port; same guard on both Windows tables (dwOwningPid == 0).
  • redactPii() now exempts the literal prefix version=" alongside ver=, so a 4-part authored version (2.2.159.0) reaches the bundle intact; new case in async_log_writer_test, and the field is listed in docs/log-redaction.md under Deliberately NOT redacted.
  • The identity line spells the peer the way the Network Diagnostics table does (::1 / ::ffff:127.0.0.1127.0.0.1) and escapes quotes, backslashes and control characters in the client-chosen name/version (a comm of x"\nINF forged logs as one field on one line).
  • Linux checks st_uid of /proc/<pid> against getuid() before reading a descriptor table — "same-user only" is now enforced on Linux and macOS; the header states that Windows is bound only by OpenProcess() rights.
  • dpkg lookups are cached per executable path + mtime.
  • /proc/net hex words are read back with qFromBigEndian/memcpy (endian-correct); the tcp/tcp6 comment names the right socket.
  • With main merged, the finished lambda uses clientStateFor() and disconnectSnapshot() carries processName/processVersion.

The log line above therefore reads TciServer: client 127.0.0.1:51234 process="wsjtx" version="3.0.1 c04dd8" on all three platforms. Verified on Linux against the built-in demo (v4, ::1, in-process client, hostile name, TIME_WAIT reuse, quit with lookups in flight).

skerker and others added 8 commits August 21, 2026 08:44
…on (aethersdr#5087)

TCI has no client-identification message and the WebSocket handshake is a
bare upgrade, so the connect log held only a peer address. For a same-
machine client the OS knows which process owns the socket; this asks it.

- New src/core/TciPeerProcess.{h,cpp}: resolveLoopbackPeerProcess() maps
  the client's local endpoint to its owning process. Linux: /proc/net/tcp6
  then /proc/net/tcp (a loopback IPv4 client on the Any-bound listener
  appears v4-mapped in tcp6) -> socket inode -> /proc/*/fd -> comm + exe.
  macOS: proc_listpids -> PROC_PIDLISTFDS -> PROC_PIDFDSOCKETINFO ->
  proc_name + proc_pidpath. Windows: GetExtendedTcpTable(OWNER_PID) for
  AF_INET and AF_INET6 -> QueryFullProcessImageNameW, version from the exe
  resource. Non-loopback peers and every failure return unresolved.
- TciServer::onNewConnection() logs today's line unchanged, then resolves
  off-thread (QtConcurrent + QFutureWatcher) so the descriptor sweep never
  delays sendInitBurst(); a second line carries process/exe/version when
  it lands. A non-loopback peer gets one qCDebug saying identity is
  unavailable. Identity is kept on ClientState/TciClientInfo.
- Network Diagnostics TCI client table: endpoint cell shows "(name)" with
  exe path (+ version) as tooltip.
- tests/tci_peer_process_test: a self-connected QTcpServer/QTcpSocket pair
  on IPv4 and IPv6 loopback resolves to the test binary; a non-loopback
  address and port 0 do not.
- CMake: new .cpp in aethercore; Windows links iphlpapi and version.

Fixes aethersdr#5087
A macOS client is usually an app bundle, and its Info.plist carries the
version the user sees (WSJT-X: "3.0.1"). Walk up from the resolved
executable to Contents/Info.plist and read CFBundleShortVersionString
(CFBundleVersion as fallback) — a plain file read, never an execution of
the client. A bare executable yields no version, as before.
…eep can match (aethersdr#5087)

QFile::symLinkTarget() absolutizes a relative-looking link target against
the link's directory, so every /proc/<pid>/fd socket entry came back as
"/proc/<pid>/fd/socket:[N]" and the inode comparison never matched — the
Linux resolver resolved nothing, ever. Found by the Linux leg of the
per-platform proof: fldigi-TCI connected and no identity line appeared;
the existing tci_peer_process_test fails 6 checks on Linux against the
old code and passes 12/12 with the raw readlink(2) comparison. macOS and
Windows use native APIs and are untouched.
…hersdr#5087)

The Windows backend read only the numeric VS_FIXEDFILEINFO quad, which
drops the build metadata authors put in ProductVersion (WSJT-X:
"3.0.1 c04dd8") and always renders masked in logs, because a 4-part
dotted version matches the log sanitizer's IPv4 rule while authored
2/3-part strings do not. Query the translation table's ProductVersion,
then FileVersion, and keep the numeric quad only as a fallback for exes
with no string table — the same authored-version semantics the macOS
backend gets from CFBundleShortVersionString.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CFBundleShortVersionString alone answers "which version" but not "which
build" — the question a support thread actually ends up asking.  Report
"3.0.1 (123)" when CFBundleVersion differs from the marketing version,
either key alone when only one is set.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hersdr#5087)

An ELF binary embeds no version, so Linux identity lines stopped at
name + exe path.  For distro-installed clients the dpkg database has
the answer and is plain text: /var/lib/dpkg/info/<pkg>.list maps the
exe path to its owning package, /var/lib/dpkg/status maps the package
to its version — which carries the packaging build, e.g.
"2.6.1+repack-2build1".  File reads only, never a package tool;
home-built binaries match no .list and stay version-less; rpm's
database is sqlite blobs, so non-dpkg distros remain a follow-up.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntry lies (aethersdr#5087)

First live Windows run still fell back to the masked numeric quad: a
version-resource probe showed WSJT-X 3.0.1 declares Translation
lang=0409 cp=004b while its strings actually live under 040904B0, so a
Translation-driven lookup alone can never find them.  Append the
standard en-US and language-neutral Unicode blocks (040904b0, 000004b0)
as fallback candidates — the same list .NET's FileVersionInfo uses for
the same reason.  Block lookup measured case-insensitive.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@skerker
skerker marked this pull request as ready for review August 24, 2026 22:26
@skerker
skerker requested review from a team as code owners August 24, 2026 22:26
@skerker

skerker commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

@AetherClaude please review

@jensenpat jensenpat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an important discovery tool for TCI applications, but changes the dynamics between our Microsoft Store publication (system process data) and privacy implications for the project. Jeremy will need to rule on the approach before this can merge.

@skerker

skerker commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@jensenpat maybe a less invasive alternative could be when the user files a bug report or issue and it involves TCI - aetherSDR could prompt/remind the user to provide version and build info for connected apps (like WSJT-X). Just a thought.

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue fit

#5087: TCI has no client-identification message and the handshake is a bare WebSocket upgrade, so a support bundle held nothing but client connected from "::ffff:127.0.0.1". For same-machine clients the OS knows the owning process, and asking it is the only way to get that answer. Three platform implementations, each going to the primary source rather than guessing.

Verified empirically on macOS/arm64: the full AetherSDR target builds clean and tci_peer_process_test passes (0.35 s).

No blockers. One thing for @ten9876 that is not this PR's defect but lands here, and some things worth recording as correct.

Principle VII holds

/proc/net/tcp6 parsing is bounds-checked at every step — col.size() < 10 before indexing col[1], loc.size() != 2 before loc[0]/loc[1], toUShort(&ok, 16) with the ok flag actually tested rather than discarded. That is the right posture for text that could be truncated or reformatted by a kernel change.

Two details that clearly came from measurement rather than documentation, and both have their evidence in the comment:

  • 26d7ebfbQFile::symLinkTarget() absolutizes a relative-looking target, so /proc/<pid>/fd/N's raw socket:[N] comes back as /proc/<pid>/fd/socket:[N] and can never match the inode tag. Comparing by raw readlink is the fix, and "measured" in the comment is doing real work there.
  • b3d25350 — real Windows executables ship Translation entries that do not match the block their strings actually live in, so trying the declared pairs plus 040904b0/000004b0 is what makes the version lookup work in practice. Preferring ProductVersion over FileVersion because that is where build metadata lives (WSJT-X's reads "3.0.1 c04dd8") is the kind of thing only a bench session tells you.

The IPv4-appears-v4-mapped-in-tcp6 note is also the right level of detail for the next person, since it explains why both files are read rather than one.

Scope

Everything is explained by #5087. No CHANGELOG.md entry — correct. The NetworkDiagnosticsDialog change is the minimum consumer — endpoint plus process name inline, exe path and version in the tooltip.

For the maintainer, not a change request

This logs third-party install paths, and redactPii() has no rule for filesystem paths. (inline: TciServer.cpp:765) The qCInfo emits exe="<full path>", every log line passes through redactPii() (AsyncLogWriter.cpp:122), and that function covers IPv4, radio serials, auth tokens, first_name/last_name-style fields, GPS coordinates and MAC addresses — not paths.

I checked whether this is new exposure before writing it up, and it mostly is not: QsoRecorder, CatPort, EibiClient, ThemeManager and others already log paths that sit under the user's home, so the redactor has never covered this and the practice is established. So I am not asking you to change anything here.

What is new is the source. Until now the app logged its own paths; this logs the install path of whatever else is running on the machine, and on Windows a per-user install is routinely C:\Users\<name>\AppData\Local\…. Given redactPii() exists because of GHSA-ccrg-j8cp-qhc4 and already trims a MAC to its last octet, an OS username arriving by a new route seems worth a deliberate yes or no rather than arriving as a side effect. A homePath()-prefix rule in redactPii() would cover this and the pre-existing sites together — that is a separate issue, and I would rather it were filed than folded in here.

Nits

  • resolveLoopbackPeerProcess() reads /proc/*/fd for every PID until it finds the inode. For processes owned by another user that fails with EACCES per-entry, which is correct and silent — worth one line saying so, since a reader wondering why there is no permission check deserves the answer.
  • The macOS path walks proc_listpids then PROC_PIDLISTFDS per process. Same shape, same cost, and it runs once per connection rather than per message, so no concern — noting only that a machine with many processes pays it on every TCI connect.

Verified vs. read

Built and ran: the full app target and tci_peer_process_test. Traced: redactPii()'s full rule set and its application point, plus the existing path-logging sites that establish the practice. Read: the /proc parser bounds and the fd-comparison fix. Not verified: no Linux or Windows host here, so the /proc walk, the GetExtendedTcpTable path and the StringFileInfo block fallback all rest on your measurements — and those are the three that could not have been reasoned out.

Comment thread src/core/TciServer.cpp Outdated
@ten9876

ten9876 commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

What process-level data this PR actually collects, and how to minimize it

Short answer. The PR records three things about the one process that connected to the TCI port: its short name, its full executable path, and a best-effort version. Nothing is retained about any other process. The path is the only field with a privacy cost, and it can be dropped or trimmed without losing the diagnostic value. The lookup mechanism does enumerate other processes' descriptors on Linux and macOS to find the match, and that is the part behind the "system process data" concern.

What is retained (per connected loopback client, in the log and the Network Diagnostics tooltip):

Field Example Source
name wsjtx /proc/<pid>/comm, proc_name, or the exe basename
exe path C:\Users\<name>\AppData\Local\Programs\wsjtx\bin\wsjtx.exe /proc/<pid>/exe, proc_pidpath, QueryFullProcessImageNameW
version 3.0.1 c04dd8 exe resource strings (Windows), Info.plist (macOS), dpkg status file (Linux)

Remote peers get nothing. No command line, environment, user, PID, or window title is read. Nothing is executed. The log is local and only leaves the machine if the user shares a bundle.

What is touched but not retained, which is where the platforms differ:

  • Windows asks the kernel for the TCP connection table with owning PIDs, finds the one row matching the client port, and opens that single process with PROCESS_QUERY_LIMITED_INFORMATION. It never looks at any other process. This is the same call netstat -o makes.
  • Linux reads the socket inode from /proc/net/tcp{,6}, then walks every /proc/<pid>/fd/ it can read (in practice, same-user processes only) comparing link targets until one matches. It stops at the first hit.
  • macOS lists all PIDs, then pulls the descriptor list and socket info for each until a match. This is the widest sweep.

So on Linux and macOS the code briefly enumerates the descriptor tables of unrelated processes. Nothing from that walk leaves the function, but a reader of the source sees a process sweep, and a Store reviewer reading a privacy declaration would ask whether the app "accesses information about other apps". On Windows, where the Store question actually lives, the lookup is already minimal.

How to minimize without losing usefulness. The goal in #5087 is to know which client is on the socket and, ideally, which version. In order of impact:

  1. Log the basename, not the full path. Change exe="…" to the file name only. The path's only diagnostic value is distinguishing two installs of the same client, which is rare enough to live in the tooltip rather than the log. This removes the account-name exposure entirely and costs nothing.
  2. Keep the version lookup as is. The dpkg and Info.plist reads are file reads scoped to that one executable. No change needed, but worth stating plainly in the PR body for the Store privacy declaration.
  3. Filter the macOS sweep to same-user PIDs before pulling descriptors. Linux is already effectively same-user because the fd readlink fails otherwise; making macOS match keeps the sweep smaller and makes the intent visible in code.
  4. Say what it does in the privacy text. One sentence in the Store listing and the support-bundle notice: "When a TCI client connects from this computer, AetherSDR records the connecting program's name and version in its local log."

On the prompt-the-user alternative (@skerker): asking the user to type the client version when filing a bug is what happens today, and #5087 exists because it fails in practice. Users report "WSJT-X" without the build, and the interesting cases are the ones where they are not sure what connected at all.

Ruling. Approach is approved with two changes: log the basename only (keep the full path in memory for the tooltip if you like, but never log it), and add the macOS UID filter. That turns the log line into process="wsjtx" version="3.0.1 c04dd8", which is all a maintainer needs, and reduces the Store question to "reads the name and version of the program that connected to it".

skerker and others added 2 commits September 7, 2026 22:59
…able path (aethersdr#5087). Principle VII.

The client-identity line carried exe="<full path>". A per-user install
(Windows: C:\Users\<name>\AppData\Local\...) puts the OS account name in
every support bundle, and the path adds nothing to the question aethersdr#5087
asks — which client, which version. The line is now
process="wsjtx" version="3.0.1 c04dd8". The path stays in memory for the
Network Diagnostics tooltip, in-app only.

Maintainer ruling on aethersdr#5130 (2026-09-07). No test: asserting on this line
needs a live TciServer and a WebSocket client, which is a socket-based
test; the bridge run in the PR body is the proof.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KBNAMYy5ixbhenCpDfqqqV
…ersdr#5087). Principle VII.

resolveMac() listed every pid (PROC_ALL_PIDS) and relied on other users'
processes refusing the fd listing. It now asks libproc for this uid's
pids only (PROC_UID_ONLY, getuid()), so the sweep covers exactly what it
can read and the same-user intent is visible in the code — the Linux
sweep already behaves that way through unprivileged readlink. Windows
never sweeps (kernel connection table, one process opened).

Maintainer ruling on aethersdr#5130 (2026-09-07). tci_peer_process_test's
self-connection is same-uid and still resolves.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KBNAMYy5ixbhenCpDfqqqV
@skerker

skerker commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Both changes pushed on a2867148, head d1e69c7e, no rebase:

  • ad427370 — the identity line is now process="wsjtx" version="3.0.1 c04dd8"; exe= is gone from the log. The path stays in memory for the Network Diagnostics tooltip only, with the reason beside the log call.
  • d1e69c7eresolveMac() lists this uid's pids (proc_listpids(PROC_UID_ONLY, getuid(), …)) instead of every pid, so the sweep covers exactly what it can read and the same-user intent is in the code. Linux is unchanged (same-user by unprivileged readlink); Windows never sweeps.

Items 2 and 4 are in the body: a "What is collected" paragraph stating that the version comes from file reads scoped to the one connecting executable, nothing executed, and your one-sentence privacy text quoted for the Store listing. The support-dialog text is untouched; if you want that sentence in the app as well, say so and it is one more small commit.

Context only: #5481 landed the home-directory → ~ rule in redactPii() on main, so a path that reaches a log from any site is now masked there as well; this PR no longer logs one.

Proof on the new head: clean RelWithDebInfo build (macOS Intel, Qt 6.8.3), About v26.8.3 (d1e69c7e) over the bridge; the bare-WebSocket Python client from the 08-24 bench on port 50001 → TciServer: client ::ffff:*.*.*. 1:56564 process="Python" version="3.14.4", and exe= appears nowhere in the log; Network Diagnostics → TCI Clients cell 127.0.0.1:56564 (Python) (the tooltip on that cell, read by eye on a third run since a table cell's tooltip is not bridge-addressable — #5503: the executable path and version 3.14.4, in-app only); tci_peer_process_test 12/12 with the uid filter; File → Quit with the Network dialog open → orderly shutdown trace, no crash report. Bundle aethersdr-pr5130-ruling-revision-2026-09-08.zip, sha256 c8fdb2a4b6b76f2d1539c28683a62aef7d7db91aaf1200efaabf757993b8da4b (attached on the PR body).

tests/tci_peer_process_test.cpp is unchanged; it is the loopback self-connect test the socket-test discussion covers, and its disposition stays with the maintainers.

— authored by agent (Claude Code) on behalf of @skerker

skerker and others added 2 commits September 8, 2026 08:12
…ps with exit 77 (aethersdr#5087). Principle VIII.

The AGENTS.md test-layer boundary lets a socket-owning test stand when our
own code is the subject, provided it is visible: the tests.cmake block now
names the socket (an ephemeral-port QTcpServer on 127.0.0.1 and ::1 with a
same-process QTcpSocket client — the kernel lookup under test needs a real
socket; no peer process, no fake firmware) and registers SKIP_RETURN_CODE 77.

A loopback listen that fails used to print [SKIP] and count as a pass. It is
now reported and the run exits 77 after the remaining legs, so ctest shows
"Skipped" instead of a green result that proved nothing. Any failed check
still exits 1. Measured: forcing the IPv6 leg onto a non-local address gives
exit 77 and ctest "***Skipped"; restored, 12/12 and exit 0.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M12LaQeTs7jjf49Xq3ovX5
…les are the proof (aethersdr#5087). Principle VIII.

The test's only unique claim — a live loopback socket resolves to its
owner — was a self-connected QTcpServer/QTcpSocket pair, a socket-owning
test in the default graph. The same claim is proven on the PR with real
clients (WSJT-X, fldigi, a bare-WebSocket Python client) on Linux, macOS
and Windows through the automation bridge, which is where AGENTS.md
routes positive convergence. Its two negative checks (non-loopback peer,
port 0) still pass with the guard they name deleted — nothing on the box
has a socket with that local endpoint — so they pinned nothing.

Removes the target, its tests.cmake block and the exit-77 skip added in
74a92bf.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M12LaQeTs7jjf49Xq3ovX5
@skerker

skerker commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

9476eb04 removes the unit test tests/tci_peer_process_test.cpp. It was a socket-owning test: it opened a loopback socket to itself to check that the lookup names the owning process. That check is already covered by the bridge proof on this PR, which does the same thing with real clients on Linux, macOS and Windows. Its other two checks (a remote address and port 0 never resolve) still passed with the guard they were meant to protect removed, so they were not catching anything. The PR now carries no unit test; the evidence bundles above are the proof.

— authored by agent (Claude Code) on behalf of @skerker

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue fit

#5087 asks for the owning local process (name, exe, best-effort version) behind each loopback TCI connection, resolved by the OS, degrading to today's line on any failure. The diff does that on all three platforms, off the accept path, with the two changes from the 2026-09-07 ruling applied (exe= out of the log, macOS sweep limited to this uid). Verified on Linux (this review's build, Arch, Qt 6.10): a v4 loopback client, a ::1 client and the in-process bridge client all resolve about 20 ms after connect, the Network Diagnostics cell reads 127.0.0.1:44268 (python3), and 45 rapid connect/close cycles plus a quit with lookups in flight produced no crash and an orderly shutdown trace.

No merge blockers by the rubric (nothing breaks users, violates canon, or fails on main). Two reproduced defects in the feature's own purpose are cheap to fix and I would want them in before merge; they are inline with suggestions.

Test-boundary preflight

The net diff adds no test and touches neither tests.cmake nor any socket-owning source. The branch's own tci_peer_process_test (self-connected QTcpServer) was added and then removed in 9476eb04 before reaching main, so nothing gated execution. Noting the removal here per AGENTS.md; reviewed normally.

Scope

File / group What it changes Claimed? Verdict
src/core/TciPeerProcess.{h,cpp} (new) OS socket→process resolver, three backends, version lookups Yes In scope
src/core/TciServer.{h,cpp} async lookup after connect, identity on ClientState/TciClientInfo, second log line Yes In scope
src/gui/NetworkDiagnosticsDialog.cpp endpoint cell suffix + tooltip Yes In scope
CMakeLists.txt new TU in CORE_SOURCES; iphlpapi version on Win32 Yes In scope

All 12 commits are dated 2026-08-21 to 2026-09-08 and every message names #5087. No CHANGELOG entry (correct). Nothing removed on the - side except the dialog's plain endpoint text, which nothing else read (the alias key is UserRole on column 0). New public surface: none (no protocol verb, no settings key). The body's checklist holds.

Findings (reproduced, non-blocking, fix recommended)

1. A stale TIME_WAIT row with the same local port aborts the Linux lookup (inline: TciPeerProcess.cpp:100, same shape on Windows at :373/:392). findSocketInode() returns the first row whose local endpoint matches and resolveLinux() treats inode == 0 as terminal, but a TIME_WAIT/orphaned row for that port carries inode 0 and can precede the live row. Reproduced 6/6 on this box with a client that reused its source port for a second local connection:

port 40501 rows for local port in /proc/net/tcp (order as listed):
  local 0100007F:9E35 rem 0100007F:C3D3 st 06 inode 0
  local 0100007F:9E35 rem 0100007F:C3D2 st 01 inode 1222178

Those six connections were held 1.5 s each (the lookup takes ~20 ms) and logged client connected from but never a process= line; 39 connect lines total, 3 identity lines. Skipping zero-inode rows (continue instead of return) fixes it; on Windows the analogue is dwOwningPid == 0 for TIME_WAIT rows (reasoned from the API, not run — no Windows host here).

2. A 4-part authored version string is masked by the log sanitizer (inline: TciServer.cpp:772, comment at TciPeerProcess.cpp:310). The IPv4 rule's exemption is the literal lookbehind (?<!ver=); version=" does not satisfy it. Run against the rule at AsyncLogWriter.cpp:274:

version="3.0.1 c04dd8"  -> version="3.0.1 c04dd8"   (survives)
version="2.2.159.0"     -> version="*.*.*. 0"       (JTDX-style ProductVersion)
version="4.2.6.0"       -> version="*.*.*. 0"

So the comment's "authored strings are not [masked]" holds only for non-quad strings; any Windows client whose ProductVersion is a dotted quad (common) logs version="*.*.*. 0" in the bundle, which is the symptom the PR set out to remove. docs/log-redaction.md:54 lists "software version (including 4-part build numbers)" under Deliberately NOT redacted, so the right-depth fix is one more lookbehind in AsyncLogWriter.cpp:274 ((?<!ver=)(?<!version=")) plus a case in tests/async_log_writer_test.cpp next to the existing software_ver= one. Renaming the field will not do it: the lookbehind needs ver= immediately before the digits.

Nits (non-blocking)

  • The tcp6-first comment describes the wrong row (inline TciPeerProcess.cpp:82). The client's own AF_INET socket lives in /proc/net/tcp; it is our accepted socket that appears v4-mapped in tcp6. Observed live: client 0100007F:A9D0 only in tcp, the FFFF00000100007F:C3D2 row in tcp6 is ours. The code works via the fall-through; the comment invites someone to drop the second file.
  • parseProcNetAddress is little-endian-host-only (inline :62). qFromBigEndian<quint32>(w) and a memcpy of the native word say what the kernel actually printed and are correct on both endiannesses. Low practical impact.
  • "Same-user only" is enforced in code on macOS only. On Linux it is a side effect of unprivileged readlink; an AetherSDR run as root or with CAP_SYS_PTRACE resolves other users' clients, and an elevated Windows process can open other sessions' processes. Worth one honest clause in the body's What is collected paragraph, or a st_uid == getuid() check before the fd walk if the guarantee is meant literally.
  • docs/log-redaction.md "Deliberately NOT redacted" does not list the new process=/version= fields; one bullet citing #5087 and the ruling keeps a future redaction rule from silently eating them.
  • info.name is client-controlled (/proc/<pid>/comm via prctl(PR_SET_NAME)) and is spliced into a .noquote() line; a name containing " or \n can forge a record. Same-user process, so bounded; escaping quotes/control characters closes it.
  • dpkg sweep per connect. Every loopback connect re-reads all /var/lib/dpkg/info/*.list (thousands of files) on the global QtConcurrent pool. A static QHash<QString,QString> keyed by exe path (mtime-invalidated) bounds it to once per client binary.
  • Main has moved under the base: origin/main now has TciServer::clientStateFor(QWebSocket*) and disconnectSnapshot(). The lambda's hand-rolled m_clients scan becomes a second copy of the helper after merge, and the disconnect snapshot will not carry processName/processVersion. Merge is conflict-free; consider folding both in.
  • Squash message: the PR title is 79 characters and lacks the Principle <N>. suffix AGENTS.md asks for; the merger can trim it.

What I verified vs. read

Built and drove the PR head (9476eb04) in a scratch worktree, offscreen, isolated AETHER_SETTINGS_DIR, connected to the built-in demo DEMO-0001 ("AetherSDR Demo", family sim), TCI server on port 50130, AETHER_AUTOMATION_NO_TX=1. Clients: a bare-upgrade Python WebSocket client over 127.0.0.1 and ::1, and the bridge's in-process tci start sim (resolves to process="AetherSDR"). Read back via the file log and a grab of the Network Diagnostics TCI Clients page. Tried and held: 30 zero-hold connect/close cycles twice (guard path, no crash, no line for gone sockets); quit with 15 lookups in flight (phase=application event=complete); v6 and v4-mapped matching in sameHost; the /proc/net/tcp column layout. Read only: the macOS and Windows backends, the StringFileInfo block fallback, and the uid filter — no host here, so those rest on the author's bundles. Not testable here: the dpkg path (Arch), and the root/CAP_SYS_PTRACE case.

One observation for the maintainer rather than the author: aether.cat defaults to QtWarningMsg (LogManager.cpp:26), so on a fresh store neither the existing client connected from line nor the new identity line reaches the log until the operator enables the "TCI / CAT / rigctld" category. Pre-existing, and #5087's own bundle evidently had it on, but it bounds how often this lands in a bundle unasked.

Automated pass (/code-review, medium) ran; findings above that came from it were each re-verified here, the rest dropped.

Instance driven: my own offscreen PR build against the demo simulator only; closed at the end of the session.

— Claude Code review on behalf of @ten9876

Comment thread src/core/TciPeerProcess.cpp Outdated
Comment thread src/core/TciPeerProcess.cpp
Comment thread src/core/TciPeerProcess.cpp
Comment thread src/core/TciServer.cpp Outdated
Comment thread src/core/TciPeerProcess.cpp Outdated
Comment thread src/core/TciPeerProcess.cpp Outdated
Comment thread src/core/TciPeerProcess.cpp Outdated
Comment thread src/core/TciPeerProcess.cpp Outdated
Comment thread src/core/TciServer.cpp Outdated
Comment thread src/gui/NetworkDiagnosticsDialog.cpp
ten9876 and others added 2 commits September 10, 2026 09:37
…names (aethersdr#5087). Principle VIII.

Review round on aethersdr#5130 — every finding and nit from the 2026-09-10 pass,
verified on the fixed build against the built-in demo:

- findSocketInode(): a TIME_WAIT/orphaned row for the same local port
  carries inode 0 and can be listed before the live row (measured 6/6 with
  a client that reused its source port: no identity line ever). Skip zero
  inodes instead of returning them. Same guard on the Windows tables, where
  TIME_WAIT rows report owning pid 0. After the fix the same six connections
  all resolve.
- The log sanitizer's IPv4 rule now also exempts the literal prefix
  version=", so a 4-part authored ProductVersion ("2.2.159.0", JTDX-style)
  reaches the bundle intact instead of as "*.*.*. 0". Quoting alone still
  exempts nothing (existing test kept); new test pins the field spelling.
  docs/log-redaction.md lists the new fields under Deliberately NOT
  redacted.
- The identity line normalises the peer address the way the Network
  Diagnostics table does (::1 and ::ffff:127.0.0.1 -> 127.0.0.1), so a
  bundle line and a dialog screenshot name one client one way, and escapes
  quotes, backslashes and control characters in the client-chosen process
  name and version (measured: comm x"\nINF forged logs as one field on one
  line).
- Linux sweep checks st_uid of /proc/<pid> against getuid() before reading
  a descriptor table, so "same-user only" is enforced rather than a side
  effect of unprivileged readlink (root / CAP_SYS_PTRACE instances). The
  header states the per-platform contract, including that Windows is bound
  only by OpenProcess rights.
- dpkg lookup is cached per executable path + mtime; a reconnecting client
  no longer re-reads every /var/lib/dpkg/info/*.list.
- parseProcNetAddress() reads each /proc/net word back as a native integer
  (qFromBigEndian / memcpy), correct on either endianness; the tcp6/tcp
  comment now describes which socket lives in which file.
- Merged origin/main: the finished lambda uses clientStateFor(), and
  disconnectSnapshot() carries processName/processVersion so the last
  disconnect record says which program went away.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ten9876 ten9876 changed the title feat(tci): log which local process is behind each TCI client connection (#5087) feat(tci): log the process behind local TCI clients. Principle VIII. Sep 10, 2026
@ten9876
ten9876 dismissed jensenpat’s stale review September 10, 2026 16:54

Ruled 2026-09-07: approach approved with basename-only logging and the macOS uid filter, both landed (ad42737, d1e69c7); the 2026-09-10 review round is on top (7ff41b5). Dismissing the pending-ruling block so the approval can carry the merge.

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at 7ff41b5e

Second pass on the fixed head. Everything from the 2026-09-10 review landed in one commit on top of a clean merge of origin/main, and each item was re-verified on a fresh build against the built-in demo simulator (offscreen, isolated settings, TX pinned off):

  • TIME_WAIT reuse: the same six source-port-reuse connections that resolved 0/6 before now resolve 6/6.
  • 4-part versions: async_log_writer_test passes with the new version="…" exemption case; the quoted-IP-still-redacted case is unchanged.
  • Name escaping: a client with comm set to x"\nINF forged logs as one field on one line.
  • Address spelling: the ::1 client logs as 127.0.0.1:<port>, matching the Network Diagnostics cell.
  • v4, ::1 and the in-process bridge client still resolve in ~20 ms; quit with lookups in flight shuts down cleanly.

All review threads are resolved with a note on what landed. The stale pending-ruling block from before the 2026-09-07 ruling is dismissed. macOS and Windows backends compile in CI; their runtime behaviour rests on the author's three-platform bundles.

@ten9876
ten9876 enabled auto-merge (squash) September 10, 2026 16:54
@ten9876
ten9876 merged commit df0520a into aethersdr:main Sep 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

enhancement: log which local process is behind each TCI client connection — name, executable, version when discoverable

3 participants